Skip to content

linux_testing: enable Rust by default if available#345980

Merged
alyssais merged 3 commits intoNixOS:staging-nextfrom
Ma27:linux-testing-rust-aarch64
Oct 12, 2024
Merged

linux_testing: enable Rust by default if available#345980
alyssais merged 3 commits intoNixOS:staging-nextfrom
Ma27:linux-testing-rust-aarch64

Conversation

@Ma27
Copy link
Member

@Ma27 Ma27 commented Oct 2, 2024

The first assumption[1] we had was that the aarch64-unknown-none target was missing from rustc and that this was the cause for the regression.

However, it turns out that the relevant code from rustc wasn't used anyways because the Makefile does --sysroot /dev/null[2] which prevents rustc from using its own libcore. So luckily we don't have to patch the Rust bootstrap to get aarch64-linux back working[3].

In fact, the Rust part seems broken for both 6.10 and 6.11[4], but I decided to not bother since none of those are longterm versions.

So what's left here?

  • Enabling Rust for aarch64-linux because it clearly works[5].

  • Turning off NFS_LOCALIO for aarch6y4-linux because breaks the build like this:

    /nix/store/f3k0rdhcd2cx57phx755c2xixgifw5m5-binutils-2.42/bin/ld: Unexpected GOT/PLT entries detected!
    /nix/store/f3k0rdhcd2cx57phx755c2xixgifw5m5-binutils-2.42/bin/ld: Unexpected run-time procedure linkages detected!
    /nix/store/f3k0rdhcd2cx57phx755c2xixgifw5m5-binutils-2.42/bin/ld: fs/nfs/localio.o: in function `nfs_local_iocb_alloc':
    /build/source/build/../fs/nfs/localio.c:290:(.text+0x324): undefined reference to `nfs_to'
    [...]
    

[1] #315121 (comment) [2] https://lore.kernel.org/all/20231031201752.1189213-1-mmaurer@google.com/ [3] Of course I only realized this after I spent some time hacking a rustc
patch together 🙃
[4] This broke with

    error[E0463]: can't find crate for `core`
      |
      = note: the `aarch64-unknown-none` target may not be installed
      = help: consider downloading the target with `rustup target add aarch64-unknown-none`
      = help: consider building the standard library from source with `cargo build -Zbuild-std`

[5] While the build is fine, the VM tests are still panicking, but
that's also the case for kernel-generic because of a 9p
regression:

    switch_root: can't execute '/nix/store/zv87gw0yxfsslq0mcc35a99k54da9a4z-nixos-system-machine-test/init': Exec format error
    [    1.734997] Kernel panic - not syncing: Attempted to kill init! exitcode=0x00000100
    [    1.736002] CPU: 0 UID: 0 PID: 1 Comm: switch_root Not tainted 6.12.0-rc1 #1-NixOS
    [...]

Reported as https://lore.kernel.org/all/D4LHHUNLG79Y.12PI0X6BEHRHW@mbosch.me/T/#u

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 24.11 Release Notes (or backporting 23.11 and 24.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@Ma27 Ma27 requested review from K900, alyssais and winterqt October 2, 2024 17:23
@github-actions github-actions bot added the 6.topic: kernel The Linux kernel label Oct 2, 2024
@K900
Copy link
Contributor

K900 commented Oct 2, 2024

What happens on other targets now that we unconditionally enable Rust?

@winterqt
Copy link
Member

winterqt commented Oct 2, 2024

Could we just enable it on aarch64 and x86_64? I'm not sure if it even supports other arches rn/if we can easily test other arches.

@Ma27 Ma27 force-pushed the linux-testing-rust-aarch64 branch from 2619a51 to fec2614 Compare October 2, 2024 21:26
@Ma27
Copy link
Member Author

Ma27 commented Oct 2, 2024

Yeah good point, was too focused on getting the aarch64 build done :)

@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux. labels Oct 3, 2024
@Ma27 Ma27 mentioned this pull request Oct 3, 2024
13 tasks
@alyssais
Copy link
Member

alyssais commented Oct 3, 2024

Could we just enable it on aarch64 and x86_64? I'm not sure if it even supports other arches rn/if we can easily test other arches.

Generally we don't limit functionality for other architectures just because we can't test it — I'm not aware of any other architecture-specific issues, so I think this should be enabled in all cases where we have a Rust compiler available, assuming we can cross compile kernels for a couple of other architectures. (See gstreamer for an example of how to do this check.) Otherwise, other architectures lag behind, and things fail on them for confusing and unnecessary reasons due to this sort of divergence.

@Ma27
Copy link
Member Author

Ma27 commented Oct 6, 2024

I pushed a potential fix by making the support for CONFIG_RUST by dsefault dependant on whether rustc supports the host platform.

For testing purposes, I decided to build it for powerpc64le (built successfully) and riscv64 which failed to build with

error: unknown argument: '-mno-riscv-attribute'
error: unsupported argument 'medany' to option '-mcmodel=' for target 'unknown'
error: unsupported option '-fpatchable-function-entry=4' for target 'unknown'
error: unknown target triple 'unknown'

This is known and apparently riscv specific: https://lore.kernel.org/lkml/31885EDD-EF6D-4EF1-94CA-276BA7A340B7@kernel.org/T/
Current recommendation is to not mix up GCC and rustc apparently. If you want me to, I can fix this in the upcoming days, but I'm not sure if we should delay a fix for a aarch64-linux, a tier1 architecture for that.

Anyways, is this (i.e. availableOn) the direction we want to go? cc @alyssais

@vcunat vcunat changed the title linux_testing: enable Rust by default for aarch64-linux linux_testing: enable Rust by default if available Oct 7, 2024
@alyssais
Copy link
Member

alyssais commented Oct 9, 2024

Current recommendation is to not mix up GCC and rustc apparently. If you want me to, I can fix this in the upcoming days, but I'm not sure if we should delay a fix for a aarch64-linux, a tier1 architecture for that.

I wouldn't worry about this one for now.

@alyssais
Copy link
Member

alyssais commented Oct 9, 2024

Although I guess if we're fixing the conditional anyway, might as well add a !(stdenv.hostPlatform.isRiscV && stdenv.cc.isGNU) check…

@Ma27 Ma27 force-pushed the linux-testing-rust-aarch64 branch from d5bfe60 to 68e45b4 Compare October 12, 2024 08:58
Ma27 and others added 3 commits October 12, 2024 16:41
The first assumption[1] we had was that the `aarch64-unknown-none`
target was missing from rustc and that this was the cause for the
regression.

However, it turns out that the relevant code from `rustc` wasn't used
anyways because the Makefile does `--sysroot /dev/null`[2] which prevents
rustc from using its own libcore. So luckily we don't have to patch the
Rust bootstrap to get aarch64-linux back working[3].

In fact, the Rust part seems broken for both 6.10 and 6.11[4], but I
decided to not bother since none of those are longterm versions.

So all that's left here is to enable Rust for aarch64-linux because it
clearly works[5].

[1] NixOS#315121 (comment)
[2] https://lore.kernel.org/all/20231031201752.1189213-1-mmaurer@google.com/
[3] Of course I only realized this _after_ I spent some time hacking a rustc
    patch together 🙃
[4] This broke with

        error[E0463]: can't find crate for `core`
          |
          = note: the `aarch64-unknown-none` target may not be installed
          = help: consider downloading the target with `rustup target add aarch64-unknown-none`
          = help: consider building the standard library from source with `cargo build -Zbuild-std`

[5] While the build is fine, the VM tests are still panicking, but
    that's also the case for `kernel-generic` because of a 9p
    regression:

        switch_root: can't execute '/nix/store/zv87gw0yxfsslq0mcc35a99k54da9a4z-nixos-system-machine-test/init': Exec format error
        [    1.734997] Kernel panic - not syncing: Attempted to kill init! exitcode=0x00000100
        [    1.736002] CPU: 0 UID: 0 PID: 1 Comm: switch_root Not tainted 6.12.0-rc1 #1-NixOS
        [...]

    Reported as https://lore.kernel.org/all/D4LHHUNLG79Y.12PI0X6BEHRHW@mbosch.me/T/#u
This breaks the build like this:

      /nix/store/f3k0rdhcd2cx57phx755c2xixgifw5m5-binutils-2.42/bin/ld: Unexpected GOT/PLT entries detected!
      /nix/store/f3k0rdhcd2cx57phx755c2xixgifw5m5-binutils-2.42/bin/ld: Unexpected run-time procedure linkages detected!
      /nix/store/f3k0rdhcd2cx57phx755c2xixgifw5m5-binutils-2.42/bin/ld: fs/nfs/localio.o: in function `nfs_local_iocb_alloc':
      /build/source/build/../fs/nfs/localio.c:290:(.text+0x324): undefined reference to `nfs_to'
      [...]

Reported as https://lore.kernel.org/all/D4OUJRP8YWRM.ATQ7KASTYX5H@mbosch.me/
* It doesn't matter if `rustc` is available, but if rustc can compile to
  the hostPlatform. So use a custom condition instead of `availableOn`.
* Explicitly exclude the combination of GCC and riscv which is known
  to be broken[1].

[1] https://lore.kernel.org/lkml/31885EDD-EF6D-4EF1-94CA-276BA7A340B7@kernel.org/T/

Co-authored-by: Alyssa Ross <hi@alyssa.is>
@Ma27 Ma27 force-pushed the linux-testing-rust-aarch64 branch from 68e45b4 to 067f0b5 Compare October 12, 2024 14:44
@Ma27 Ma27 requested a review from alyssais October 12, 2024 14:45
@alyssais
Copy link
Member

@ofborg build linux_testing

@alyssais alyssais merged commit d3e6c8f into NixOS:staging-next Oct 12, 2024
@Ma27 Ma27 deleted the linux-testing-rust-aarch64 branch October 12, 2024 15:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.topic: kernel The Linux kernel 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants